Skip to content

fix(security): harden saved-credential, proxy egress, and keyless-mint paths from the release audit - #9865

Merged
waleedlatif1 merged 4 commits into
stagingfrom
fix/release-audit-hardening
Nov 11, 2026
Merged

waleedlatif1 merged 4 commits into
stagingfrom
fix/release-audit-hardening

Conversation

@waleedlatif1

@waleedlatif1 waleedlatif1 commented Nov 10, 2026 •

Copy link
Copy Markdown
Collaborator

Summary

Verified fixes from the audit of the current staging→main release, scoped to the items this PR owns.

Security

  • POST /api/auth/oauth/token refuses session callers resolving a saved service-account credential (403 SERVICE_ACCOUNT_SESSION_FORBIDDEN). Saved secrets resolve only in-process (internal JWT). OAuth, managed OAuth, and internal-JWT callers are unchanged; the only callers are the route and resolveExecutorCredentialToken (always INTERNAL_JWT)
  • Egress through HTTP(S)_PROXY: a hostname is refused when any resolved record is blocked (requireEveryAddress), since the proxy re-resolves and may dial the blocked one. IP literals and NO_PROXY hosts are unchanged
  • mintWorkloadIdentityToken refuses a subject that does not belong to the minting owner (assertWorkloadIdentitySubjectOwner), covering AWS and Databricks in one place
  • Twilio list calls: recordings URLs must resolve on https://api.twilio.com (saved Basic auth rides along); per-call recordings fetches are bounded to 10 in flight
  • refuseUnsafeTypedUrl refuses the single-slash https:/host form, like normalizeHttpBaseUrl; Dynatrace /api/v1|v2 suffix strips are case-insensitive

Correctness

  • sim-setup reset refuses when Docker is installed but unreachable, instead of archiving env files while the volumes survive

Cleanups

  • deployments.ts uses the shared requireMutableWorkflow

Docs

  • sim-setup README: dev-mode loopback binding and how to rebind older containers, no port 5432 requirement for Compose, reset order
  • Helm: writable emptyDir mounts for read-only root (/tmp, .next/cache, .next/server/route-cache)
  • Networking: AWS proxy row (STS inherits, SSO does not); every-address refusal under a proxy
  • GitLab permission CSV describe names the 10 MiB request body limit (OpenAPI regenerated)

The crawl-depth and proxied response-cap fixes (and the CONTRIBUTING update) landed in #9860. #9859 owns the Databricks response caps, #9858 the workflow middleware and route-policies cleanups, and #9862 Firecrawl search.

Type of Change

  • Bug fix
  • Security fix
  • Documentation

Testing

Each security regression test was run against the code with its guard removed and failed, then passed with the guard restored:

  • app/api/auth/oauth/token/route.test.ts: a session caller with a service-account credential gets 403 and no secret; internal-JWT resolution and session OAuth still return tokens
  • lib/core/network/environment-proxy.server.test.ts: a name resolving to 93.184.216.34 + 169.254.169.254 is refused with zero proxy tunnels, while all-public records still tunnel
  • lib/credentials/workload-identity/issuer.test.ts: minting for another workspace, an organization, or a lookalike id (ws-10) throws
  • lib/credentials/token-service-accounts/validators/http.test.ts: refuseUnsafeTypedUrl('https:/evil.com') throws
  • tools/twilio_voice/list_calls.test.ts: @evil.test/x and absolute off-origin paths never fetch off Twilio; concurrency peaks at 10
  • packages/sim-setup/src/lifecycle.test.ts: with Docker unreachable, reset rejects and leaves the env file unarchived

Also ran the other touched test files (268 tests), bun run lint, check:source-conventions, check:comment-hygiene, check:test-patterns, check:utils, check:api-validation:strict, check:boundaries, check:explicit-any (baseline regenerated), check:openapi, check:cli-api, check:mcp-operations, check:cli-docs, check:v2-route-table, check:docs, check:tool-metadata, and the pre-push type-check.

Checklist

  • Code follows project style guidelines
  • Self-reviewed my changes
  • Tests added/updated and passing (new tests pass the test-audit authoring gate)
  • No new warnings introduced
  • I confirm that I have read and agree to the terms outlined in the Contributor License Agreement (CLA)

…p paths from the release audit

- Refuse session callers resolving saved service-account secrets on POST /api/auth/oauth/token
- Refuse a proxied hostname when any resolved record is blocked (the proxy re-resolves)
- Apply the response cap to every body, not only decoded ones (undici's http proxy wrapper ignores maxResponseSize)
- Bind workload-identity subjects to their owner at mint time
- Bound Databricks token/identity JSON reads; refuse single-slash https:/ in typed URLs; case-insensitive Dynatrace /api/vN strip
- Twilio recordings: stay on the Twilio API origin and bound fetch concurrency
- Firecrawl: count crawl maxDepth below the start URL; stop sending unused search sources
- sim-setup reset refuses when Docker is installed but unreachable
- Remove the dead requireDeployment branch from workflow access middleware; reuse requireMutableWorkflow; move a misplaced TSDoc
- Docs: CONTRIBUTING setup via sim-setup, sim-setup README, Helm read-only-root mounts, proxy and NO_PROXY behavior, GitLab CSV body limit
@vercel

vercel Bot commented Nov 10, 2026 •

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated
docs Ready Ready Preview Nov 11, 2026 12:12am UTC

Request Review

@greptile-apps

greptile-apps Bot commented Nov 10, 2026 •

Copy link
Copy Markdown
Contributor

RetriggerView in GreptileConfidence Score: 5/5

[High impact] The PR appears safe to merge; no new actionable issues were found.

Summary

This PR tightens saved-secret access, proxy destination checks, workload-identity subjects, and Twilio recordings requests.

  • Session callers cannot resolve saved service-account secrets.
  • Proxied requests require every DNS answer to pass the egress check.
  • Workload tokens are minted only for the matching credential owner.
  • Twilio recording lookups stay on Twilio and run ten at a time.
  • Typed service-account URLs reject single-slash HTTP schemes.
  • Dynatrace URL cleanup recognizes uppercase API suffixes.
  • sim-setup reset stops when Docker is installed but unreachable.
  • Setup docs explain loopback-only dev ports and older container rebinding.
  • Helm read-only-root guidance names the writable Next.js cache paths.
  • GitLab permission docs state the combined 10 MiB request-body limit.
  • Workflow deployments reuse the shared mutability check.

Reviews (3) · Last reviewed commit: "fix(security): stop queued Twilio record..." · Reviewed by Greptile

@cubic-dev-ai cubic-dev-ai Bot left a comment •

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

1 issue found across 39 files

Confidence score: 3/5

  • lifecycle.ts can archive environment files when Docker is unavailable, leaving volumes behind with unusable credentials. Block reset whenever Docker is unreachable.
Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="packages/sim-setup/src/lifecycle.ts">

<violation number="1" location="packages/sim-setup/src/lifecycle.ts:597">
P2: This guard still archives environment files when the Docker CLI is absent, because `executableExists('docker')` is false; block reset whenever Docker is unreachable so volumes cannot be left behind with unusable credentials.</violation>
</file>

Turn on auto-fix | Re-trigger cubic

Comment thread apps/sim/tools/firecrawl/crawl.ts Resolved
Comment thread packages/sim-setup/src/lifecycle.ts Resolved
Drops the response cap and crawl depth (landed in #9860), CONTRIBUTING (#9860), Firecrawl search sources (on hold), Databricks JSON caps (#9859), and the workflow middleware and route-policies cleanups (#9858). Drops the NO_PROXY entry-syntax docs line (owned by a separate NO_PROXY change). Generated files regenerated on current staging.
@waleedlatif1

Copy link
Copy Markdown
Collaborator Author

Release-audit coordination note for this PR's owner (from the session that owns #9864, which dropped its networking.mdx edits in favor of this PR):

Please consider adding one sentence to "Proxying guarded requests" in apps/docs/content/docs/platform/self-hosting/networking.mdx: the proxy's own egress policy must refuse private, loopback, and link-local ranges (including 169.254.169.254) and internal DNS zones. A proxied request can't keep the app's IP pinning, because the proxy re-resolves the hostname (a short-TTL rebind or split-horizon DNS). requireEveryAddress narrows this but doesn't remove it. The NO_PROXY doc line belongs to sim-9c's branch per the shared owner table.

@waleedlatif1 waleedlatif1 changed the title fix(security): harden saved-credential, proxy egress, and response-cap paths from the release audit fix(security): harden saved-credential, proxy egress, and keyless-mint paths from the release audit Nov 10, 2026
@waleedlatif1

Copy link
Copy Markdown
Collaborator Author

@greptile

@waleedlatif1

Copy link
Copy Markdown
Collaborator Author

@cubic-dev-ai review this PR

@cubic-dev-ai

cubic-dev-ai Bot commented Nov 10, 2026

Copy link
Copy Markdown
Contributor

@cubic-dev-ai review this PR

@waleedlatif1 I have started the AI code review. It will take a few minutes to complete.

@cubic-dev-ai cubic-dev-ai Bot left a comment •

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All reported issues were addressed across 24 files

Requires human review: Auto-approval blocked because this review re-detected 1 unresolved issue already reported by Cubic.

Turn on auto-fix | Re-trigger cubic

…token-route auth mocks, bump chart

- Twilio list calls takes the run's signal: queued recordings requests stop once it aborts, and fetch receives it
- Token route tests type the hybrid-auth mock through the real AuthResult and pass a route context
- Chart version 1.11.13 for the README and helper comment changes
@waleedlatif1

Copy link
Copy Markdown
Collaborator Author

@greptile

@waleedlatif1

Copy link
Copy Markdown
Collaborator Author

@cubic-dev-ai review this PR

@cubic-dev-ai

cubic-dev-ai Bot commented Nov 11, 2026

Copy link
Copy Markdown
Contributor

@cubic-dev-ai review this PR

@waleedlatif1 I have started the AI code review. It will take a few minutes to complete.

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No issues found across 25 files

Confidence score: 5/5

  • Automated review surfaced no issues in the provided summaries.
  • No files require special attention.

Turn on auto-fix | Re-trigger cubic

This branch was successfully deployed

1 active deployment
Preview — d3772f3e Deployed Nov 11, 2026 by vercel[bot]
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant